-
Notifications
You must be signed in to change notification settings - Fork 0
feat: implement HTTP server for remote UEFI shell capability #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
We don't publish DEVs .whl. |
1 similar comment
|
We don't publish DEVs .whl. |
7904675 to
b38783f
Compare
|
We don't publish DEVs .whl. |
1 similar comment
|
We don't publish DEVs .whl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a remote shell (RShell) capability that enables HTTP-based command execution between a Python client and Flask server, with specific support for UEFI shell environments.
Key changes:
- Added RShellConnection class for managing remote command execution via HTTP
- Implemented Flask-based server for handling command queues and client connections
- Created UEFI-compatible HTTP client with specialized handling for UEFI shell limitations
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| mfd_connect/rshell_server.py | Flask REST server managing command queues, health checks, and result collection |
| mfd_connect/rshell_client.py | HTTP client designed for UEFI shell with polling and command execution capabilities |
| mfd_connect/rshell.py | RShellConnection class providing remote command execution interface |
| examples/rshell_example.py | Usage example demonstrating RShellConnection functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
mfd_connect/rshell_server.py
Outdated
| def get_command_to_execute(): | ||
| """Get the next command to execute for the connected client.""" | ||
| ip_address = str(request.remote_addr) | ||
| global client_connected |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable client_connected is referenced but never defined. This will cause a NameError at runtime.
| global client_connected | |
| # global client_connected # Removed: not defined or used |
Copilot uses AI. Check for mistakes.
mfd_connect/rshell_server.py
Outdated
| print(f"Client connected: {ip_address}") | ||
| clients.append(ip_address) |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client IP address is always appended to the clients list, even if it already exists. This will create duplicate entries. The append should be inside the if block.
| print(f"Client connected: {ip_address}") | |
| clients.append(ip_address) | |
| clients.append(ip_address) |
Copilot uses AI. Check for mistakes.
mfd_connect/rshell_client.py
Outdated
| except Exception as exp: | ||
| conn.request( | ||
| "POST", | ||
| str("Exception"), |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL path should be '/post_result' instead of 'Exception' to match the server endpoint.
| str("Exception"), | |
| "post_result", |
Copilot uses AI. Check for mistakes.
| if env is not None: | ||
| logger.log( | ||
| level=log_levels.MODULE_DEBUG, | ||
| msg="Environment variables are not supported for RShellConnection and will be ignored.", | ||
| ) |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check for env is not None is duplicated. The second occurrence (lines 134-138) should be removed as it's identical to lines 128-132.
| if env is not None: | |
| logger.log( | |
| level=log_levels.MODULE_DEBUG, | |
| msg="Environment variables are not supported for RShellConnection and will be ignored.", | |
| ) |
Copilot uses AI. Check for mistakes.
| msg="Skipping logging is not supported for RShellConnection and will be ignored.", | ||
| ) | ||
|
|
||
| if expected_return_codes is not None: |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter expected_return_codes uses ellipsis (...) as default value but is checked against None. This will always be False since ellipsis is not None.
Copilot uses AI. Check for mistakes.
| timeout_string = " " if timeout is None else f" with timeout {timeout} seconds" | ||
| logger.log(level=log_levels.CMD, msg=f"Executing >{self._ip}> '{command}', {timeout_string}") |
Copilot
AI
Sep 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log message formatting is incorrect. It should be f\"Executing >{self._ip}> '{command}'{timeout_string}\" to properly concatenate the timeout information.
| timeout_string = " " if timeout is None else f" with timeout {timeout} seconds" | |
| logger.log(level=log_levels.CMD, msg=f"Executing >{self._ip}> '{command}', {timeout_string}") | |
| logger.log(level=log_levels.CMD, msg=f"Executing >{self._ip}> '{command}'{timeout_string}") |
Copilot uses AI. Check for mistakes.
b38783f to
1fea908
Compare
|
We don't publish DEVs .whl. |
1 similar comment
|
We don't publish DEVs .whl. |
|
We don't publish DEVs .whl. |
1 similar comment
|
We don't publish DEVs .whl. |
Signed-off-by: Lasota, Adrian <[email protected]>
dfe39e4 to
abcb17d
Compare
|
We don't publish DEVs .whl. |
1 similar comment
|
We don't publish DEVs .whl. |
This pull request introduces a new remote shell (RShell) connection feature, enabling remote command execution via HTTP between a Python client (including UEFI shell support) and a Flask-based server. The implementation includes the RShell connection class, a sample client, and a server for managing command queues and results.
RShell Connection Implementation
RShellConnectionclass tomfd_connect/rshell.py, providing methods to start/connect to the RShell server, execute commands remotely, and manage connection lifecycle. This class includes extensive logging and supports command execution via HTTP requests.Sample Client and Server
mfd_connect/rshell_client.py, a sample HTTP client designed to run on UEFI shell, which polls for commands, executes them locally, and posts results back to the server. It includes compatibility handling for UEFI shell's limitations.mfd_connect/rshell_server.py, a Flask REST server that manages command queues for each client, provides health checks, serves commands for execution, and collects results. The server supports multiple clients and tracks command status.Usage Example
examples/rshell_example.pydemonstrating how to use the newRShellConnectionclass to connect to a remote RShell server, execute a command, and disconnect.